Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc(etcd-maintenance): add reference to etcd-defrag CronJob #43394

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

clementnuss
Copy link
Contributor

@clementnuss clementnuss commented Oct 9, 2023

With the recent documentation of a Kubernetes CronJob for the etcd-defrag tool, I think it would be valuable to also reference that possibility in the official documentation.

Indeed, running the tool as a CronJob has several benefits:

  • simplicity of use: a typical kubeadm-deployed Cluster can use the referenced manifest as-is.
  • period/automatic defragmentation is ensured

/cc @ahrtr

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 9, 2023
@k8s-ci-robot k8s-ci-robot requested review from jpbetz and mml October 9, 2023 14:08
@k8s-ci-robot k8s-ci-robot added language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Oct 9, 2023
@netlify
Copy link

netlify bot commented Oct 9, 2023

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 311aff6
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/65255a40fde50e000841ba1c
😎 Deploy Preview https://deploy-preview-43394--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@clementnuss
Copy link
Contributor Author

✅ Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 7196489
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/6524093765e55d0008530180
😎 Deploy Preview https://deploy-preview-43394--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...
To edit notification comments on pull requests, go to your Netlify site configuration.

here is the preview: https://deploy-preview-43394--kubernetes-io-main-staging.netlify.app/docs/tasks/administer-cluster/configure-upgrade-etcd/#maintaining-etcd-clusters

image

@ahrtr
Copy link
Member

ahrtr commented Oct 9, 2023

thx @clementnuss .

Since it's my personal repo (I might donate the project to etcd-io if other etcd maintainers have no objection), so I avoid giving lgtm and approve for now although I do think the change looks good to me.

@ahrtr
Copy link
Member

ahrtr commented Oct 9, 2023

cc @neolit123

Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the tool itself.
i think this falls under third party content and it neesd the following tag outside of a note:

{{% thirdparty-content %}}

it will generate a note like this:

Note: This section links to third party projects that provide functionality required by Kubernetes. The Kubernetes project authors aren't responsible for these projects, which are listed alphabetically. To add a project to this list, read the content guide before submitting a change. More information.

guide about third party content:
https://kubernetes.io/docs/contribute/style/content-guide/#third-party-content

the k/website maintainers will provide further instructions.

Comment on lines 414 to 416
It can also be useful to run the defragmentation tool as a Kubernetes CronJob,
[as documented here](https://github.com/ahrtr/etcd-defrag/blob/main/doc/etcd-defrag-cronjob.yaml)
, to make sure that defragmentation happens regularly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't match our style guide, and it makes the note longer.

How about editing the README for https://github.com/ahrtr/etcd-defrag to mention the example manifest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to improve the style according to https://kubernetes.io/docs/contribute/style/style-guide/

It's already mentioned in the README of the etcd-defrag tool. I however feel like this will make more people aware of the possibility to run a defragmentation tool in a CronJob, which is beneficial to cluster operators.

regarding the note length, it will indeed add one sentence, but that's the purpose of documentation to make information available where it can be useful, and that doesn't come without a new sentence 😅

@tengqm
Copy link
Contributor

tengqm commented Oct 10, 2023

Instead of augmenting this page for a third party tool, I'm thinking whether we can document this at https://kubernetes.io/docs/reference/tools/.
A more general approach is to expand that tools section to cover security tools, development tools, operation tools, observability tools, networking tools, etc. We are not about to promote any of these tools. We may want to do this so that people can find relevant tools when they need help.

@clementnuss
Copy link
Contributor Author

LGTM on the tool itself. i think this falls under third party content and it neesd the following tag outside of a note:

{{% thirdparty-content %}}

it will generate a note like this:

Note: This section links to third party projects that provide functionality required by Kubernetes. The Kubernetes project authors aren't responsible for these projects, which are listed alphabetically. To add a project to this list, read the content guide before submitting a change. More information.

guide about third party content: https://kubernetes.io/docs/contribute/style/content-guide/#third-party-content

the k/website maintainers will provide further instructions.

image

the note was already there 😉

@clementnuss
Copy link
Contributor Author

Instead of augmenting this page for a third party tool, I'm thinking whether we can document this at https://kubernetes.io/docs/reference/tools/. A more general approach is to expand that tools section to cover security tools, development tools, operation tools, observability tools, networking tools, etc. We are not about to promote any of these tools. We may want to do this so that people can find relevant tools when they need help.

The reference to the third-party tool on the page was already there (thankfully, I wouldn't have found it otherwise). This PR is only about mentioning the existence of a CronJob that makes using the tool simpler and periodic.

While I agree that a "reference tools" page, a bit like some "awesome-xyz" Github repo, could be nice, I think I wouldn't have consulted it, and in my opinion this really makes sense to reference tools in the context where they can be useful.

@clementnuss
Copy link
Contributor Author

clementnuss commented Oct 10, 2023

image

Here is my updated take on this. Just one new line, hopefully more conformant to the style guide this time 🙃 @sftim

I think it would be greatly beneficial to document that possibility in this part of the documentation. For example, I wouldn't have known about etcd-defrag it it hadn't been referenced here. I also haven't seen many examples where people use Kubernetes CronJob to defragment their etcd cluster, referencing it on this page would help widen out the idea.

@tengqm
Copy link
Contributor

tengqm commented Oct 10, 2023

/label tide/merge-method-squash
Agreed to host this content in the current page at least for now.
/approve

@k8s-ci-robot k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Oct 10, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tengqm

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 10, 2023
Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this works.

I still like notes to be short.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2023
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 97f0fae37f0b5030a4b3a595f07eee07a9e21939

@windsonsea
Copy link
Member

Please run the following commands to reset the CI flow:

git reset
git commit --amend --no-edit
git push origin patch-2 -f

@k8s-ci-robot k8s-ci-robot merged commit f9ad24a into kubernetes:main Oct 10, 2023
3 checks passed
Community-Programmer pushed a commit to Community-Programmer/website that referenced this pull request Oct 13, 2023
…es#43394)

* doc(etcd-maintenance): add reference to etcd-defrag CronJob

* doc: improve style according to style guide

* chore: fix file name
aojea pushed a commit to aojea/website that referenced this pull request Oct 18, 2023
…es#43394)

* doc(etcd-maintenance): add reference to etcd-defrag CronJob

* doc: improve style according to style guide

* chore: fix file name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants